Skip to content

Make the lib fully strict typed - #136

Draft
mib1185 wants to merge 3 commits into
hthiery:masterfrom
mib1185:make-the-lib-fully-and-strict-typed
Draft

mib1185 wants to merge 3 commits into
hthiery:masterfrom
mib1185:make-the-lib-fully-and-strict-typed

Conversation

@mib1185

@mib1185 mib1185 commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

This adds mypy to the dev-requirements and also integrates it into the pre-commit configuration. Mypy is configured to only check library code, but not the tests. Mypy is used in strict mode. Finally we now distribute the package type information (according to pep 561).
Most of the adjustments were done on behalf of CoPilot, but checked in detail on my own.

Co-authored-by: Copilot <copilot@github.com>
@coveralls

coveralls commented Oct 3, 2026 •

Copy link
Copy Markdown

Coverage Status

coverage: 95.298% (-1.7%) from 96.976% — mib1185:make-the-lib-fully-and-strict-typed into hthiery:master

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The pre-commit hook weakens strict checking by ignoring missing imports and omitting the cryptography dependency.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds strict typing and PEP 561 metadata across the library while improving handling of incomplete responses.

Changes:

  • Adds mypy tooling and package type metadata.
  • Annotates the client, CLI, entities, and device types.
  • Handles missing login/XML fields and tests missing challenges.
File Description
.pre-commit-config.yaml Adds the mypy hook.
requirements_dev.txt Adds mypy and request stubs.
setup.cfg Configures strict mypy and typed package data.
pyfritzhome/​py.typed Marks the package as typed.
pyfritzhome/​cli.py Types CLI handlers and arguments.
pyfritzhome/​errors.py Types exceptions and adds login details.
pyfritzhome/​fritzhome.py Types the main client and hardens parsing.
pyfritzhome/​fritzhomedevice.py Types device construction and updates.
pyfritzhome/​devicetypes/​fritzhomeentitybase.py Types shared entity state and helpers.
pyfritzhome/​devicetypes/​fritzhomedevicebase.py Types base device attributes.
pyfritzhome/​devicetypes/​fritzhomedevicealarm.py Types alarm devices.
pyfritzhome/​devicetypes/​fritzhomedeviceblind.py Types blind devices.
pyfritzhome/​devicetypes/​fritzhomedevicebutton.py Types buttons and XML helpers.
pyfritzhome/​devicetypes/​fritzhomedevicehumidity.py Types humidity devices.
pyfritzhome/​devicetypes/​fritzhomedevicelevel.py Types level controls.
pyfritzhome/​devicetypes/​fritzhomedevicelightbulb.py Types color and light controls.
pyfritzhome/​devicetypes/​fritzhomedevicepowermeter.py Types power measurements.
pyfritzhome/​devicetypes/​fritzhomedevicerepeater.py Types repeater devices.
pyfritzhome/​devicetypes/​fritzhomedeviceswitch.py Types switch devices.
pyfritzhome/​devicetypes/​fritzhomedevicetemperature.py Types temperature devices.
pyfritzhome/​devicetypes/​fritzhomedevicethermostat.py Types thermostat state and controls.
pyfritzhome/​devicetypes/​fritzhometemplate.py Types and validates templates.
pyfritzhome/​devicetypes/​fritzhometrigger.py Types trigger state.
tests/​test_fritzhome.py Tests missing login challenges.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .pre-commit-config.yaml

@flabbamann flabbamann left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @mib1185,
nice improvement, thank you 🎉👍.

I don't see any problems, the code changes are small and should make the code more robust.

But I would suggest to drop Python 3.9. It is EOL since almost a year and with 3.10 and up you can use the builtin union types like str | None instead of Optional[str]. I think it's the preferred way today. It's a bit easier to read and you don't need the imports. What do you think?

Comment thread pyfritzhome/devicetypes/fritzhomedevicebase.py
@mib1185

mib1185 commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

Hi @flabbamann
many thanks for your reply 🙂
I wouldn't drop python 3.9 in this PR, but we can already make use of the annotation future statement and those use the modern style annotations all over the library. I'll work on it.

Co-authored-by: Copilot <copilot@github.com>
@flabbamann

Copy link
Copy Markdown
Contributor

Ah, nice. I didn't know that 👍

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Several published types are inconsistent with runtime states and getter-to-setter workflows.

0 open findings

1 resolved since last review
Previously missed (4)

In code that hasn't changed since last review

Medium severity Model optional Fritzhome state and guard remote operations

pyfritzhome/​devicetypes/​fritzhomeentitybase.py:21

_fritz is actually None whenever the public constructor is called without fritz, but this cast tells strict mypy it is always a valid Fritzhome. Subclass methods then dereference it without checks, so the newly published typing hides a real runtime state instead of modeling it. Please make _fritz optional and guard those methods, or require a Fritzhome for objects that expose remote operations.

Medium severity Reject empty login challenges

pyfritzhome/​fritzhome.py:245

An empty <Challenge/> is also unusable but is not caught by this None check; it falls through to the MD5 path and sends a response derived from an empty challenge instead of reporting the malformed login response. Reject empty strings as well.

Medium severity Ensure getter colors are non-optional and setter-compatible

pyfritzhome/​fritzhome.py:500

This return type is incompatible with the documented set_color() workflow: get_colors() produces Sequence[str | None], while set_color() accepts only Sequence[str | SupportsInt]. Strictly typed clients therefore cannot pass a returned color to the setter, and widening the setter would be unsafe because it calls int() on the values. Validate/filter missing XML attributes and expose non-optional color tuples in both public layers.

Medium severity Normalize missing color temperatures before exposing them

pyfritzhome/​fritzhome.py:540

get_color_temps() now returns list[str | None], but set_color_temp() rejects None, so a value obtained from the getter cannot be passed to the setter under strict typing. Since the setter also executes int(temperature), normalize or reject missing value attributes here and publish list[str] consistently through the device wrapper.

🧠 Review effort: Balanced

@mib1185
mib1185 marked this pull request as draft October 10, 2026 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants